Skip to content

Adding type support for mutable vs. immutable slices#326

Draft
tahina-pro wants to merge 10 commits intomainfrom
_taramana_mutable_slice
Draft

Adding type support for mutable vs. immutable slices#326
tahina-pro wants to merge 10 commits intomainfrom
_taramana_mutable_slice

Conversation

@tahina-pro
Copy link
Member

@tahina-pro tahina-pro commented Feb 13, 2025

tl;dr Breaking changes

If you need mutable slices, you will need to change your code to use Pulse.Lib.MutableSlice instead of Pulse.Lib.Slice.

Adding type support for mutable vs. immutable slices

With FStarLang/karamel#533, Karamel is on track to supporting F* interfaces implemented in Rust. Thus, mutability analysis requires annotations on the types of such interfaces, to determine which arguments should be mut and which shouldn't. (@msprotz, please correct me if I'm wrong.)

Following a suggestion by @gebner , this PR:

  • makes Pulse.Lib.Slice.slice immutable
  • introduces Pulse.Lib.MutableSlice.slice for mutable slices (although this wouldn't preclude Karamel's whole-program analysis from refining them as immutable.)

To implement immutable slices, I duplicated Pulse.Lib.ArrayPtr as Pulse.Lib.ConstArrayPtr for const array pointers, which I extract to C const pointers. To do so, I rely on type abstraction: supported operations are the same except op_Array_Assignment and copy, which I removed from Pulse.Lib.ConstArrayPtr.

For compatibility purposes, I left interfaces unchanged as much as possible. So, there is a lot of code duplication. Maybe we want a type class for slice operations? I don't know.

I need to:

  • open a PR into Karamel to fix the MiniRust extraction of Pulse.Lib.Slice to mark them immutable, and add support for Pulse.Lib.MutableSlice.

tahina-pro added a commit to FStarLang/karamel that referenced this pull request Feb 14, 2025
repository: mtzguido/karamel
ref: dev
repository: FStarLang/karamel
ref: _taramana_mutable_slice
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops didn't realize this was still pointing to my repo. When restoring let's just go the main one.

: Lemma (timeless (pts_to x #p s))
[SMTPat (timeless (pts_to x #p s))]

val from_arrayptr (#t: Type) (a: AP.ptr t) (#p: perm) (#v: Ghost.erased (Seq.seq t)) : stt (ptr t)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use pulse vals here?

val base #t (p: ptr t) : GTot (A.array t)
val offset #t (p: ptr t) : GTot nat

instance val has_pts_to_array_ptr (t: Type) : has_pts_to (ptr t) (Seq.seq t)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should expose a pts_to function and make this transparent. Otherwise we will get unfolded projectors in the context (at least currently). It would be nice to have exactly this at some point, though.

@tahina-pro
Copy link
Member Author

tahina-pro commented Dec 15, 2025

Hi everyone. After releasing EverCBOR, I have some users who want appropriate const types in its C API. So, I am tempted to revive work on this PR.
However, now I see that this PR, as it is now, would make all slices immutable by default, which would require rewriting virtually all code based on Pulse slices.
By contrast, Low* buffers adopt the converse approach: buffers are mutable by default, and explicit use of ConstBuffer is necessary to have C const buffers. In retrospect, I increasingly believe that we should do the same for Pulse slices: have slices mutable by default as they are now, and define a ConstSlice type, based on a ConstArrayPtr type for C const arrays, and declare both of them immutable in Karamel. This would be harmless for Rust extraction of programs that still use Slice instead of ConstSlice, since Karamel's mutability analysis is stronger for Rust than for C anyway.
What do you think? Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants